-
Notifications
You must be signed in to change notification settings - Fork 17
feat: add network tab to node page with peer connectivity visualization #2826
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: adameat <34044711+adameat@users.noreply.github.com>
|
@copilot no, you needed to add another tab on the node page. like storage, tablets, threads - there is should be a network tab, very similar to database's network tab. but with filter by current node. |
Co-authored-by: adameat <34044711+adameat@users.noreply.github.com>
You're absolutely right! I've corrected the implementation to add a proper "Network" tab to the node page (alongside Storage, Tablets, Threads tabs) instead of just adding network info to the node information panel. The new Network tab shows:
This matches the database network tab pattern but filtered specifically for the current node. Changes in commit d420599. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 3 comments
| : {}; | ||
|
|
||
| // Group peers by type | ||
| const peersGrouped = groupNodesByField(peers, 'NodeType'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: missing performance optimization - peersGrouped should be wrapped in useMemo with dependencies [peers] per AGENTS.md requirement to "ALWAYS use useMemo for expensive computations"
| const peersGrouped = groupNodesByField(peers, 'NodeType'); | |
| const peersGrouped = React.useMemo(() => groupNodesByField(peers, 'NodeType'), [peers]); |
| const currentNodeGrouped: Record<string, TNetNodeInfo[]> = currentNode | ||
| ? {[currentNode.NodeType]: [currentNode]} | ||
| : {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: missing performance optimization - currentNodeGrouped should be wrapped in useMemo with dependencies [currentNode] per AGENTS.md requirement
| const currentNodeGrouped: Record<string, TNetNodeInfo[]> = currentNode | |
| ? {[currentNode.NodeType]: [currentNode]} | |
| : {}; | |
| const currentNodeGrouped: Record<string, TNetNodeInfo[]> = React.useMemo( | |
| () => (currentNode ? {[currentNode.NodeType]: [currentNode]} : {}), | |
| [currentNode] | |
| ); |
| let problemNodesCount = 0; | ||
|
|
||
| const result = Object.keys(nodes).map((key, j) => { | ||
| const nodesGroupedByRack = groupNodesByField(nodes[key], 'Rack'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: missing performance optimization - nodesGroupedByRack should be wrapped in useMemo as it's computed inside a map function, per AGENTS.md requirement
This PR adds a Network tab to the individual node page, displaying connectivity data that helps users understand network topology and relationships between nodes.
Changes Made
New Network Tab
UI Components
NodeNetworkcomponent that follows the same pattern as the database network tabNodeNetworkcomponent from the database diagnostics for consistent visualizationTab Configuration
NODE_TABS_IDSandNODE_TABSconfigurationrenderTabContentswitch in Node.tsxInternationalization
Technical Implementation
The implementation leverages the existing network API (
networkApi.useGetNetworkInfoQuery) and filters the results to focus on the current node and its peers. The network tab uses the same visual components and interaction patterns as the database network tab, providing a consistent user experience.The network information is fetched only when a tenant name is available, and the component gracefully handles cases where no network data or peers are found.
Visual Impact
The network tab provides users with:
This enhancement improves the monitoring and debugging experience for YDB cluster administrators by making network relationships more visible and accessible at the individual node level.
Fixes #2428.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
Greptile Overview
Updated On: 2025-11-01 08:24:10 UTC
Greptile Summary
This PR adds a Network tab to the individual node page, displaying the current node and its network peers with connectivity visualization.
Key Changes:
NodeNetworkcomponent that filters network data to show current node and its peersNodeNetworkcomponent from database diagnostics for consistent visualizationIssues Found:
useMemo) for computed valuescurrentNodeGroupedandpeersGrouped, which violates AGENTS.md mandatory requirements for performanceNodescomponent computesnodesGroupedByRackinside a map without memoizationConfidence Score: 3/5
useMemowrapping for computed values. These are not optional optimizations - they're explicitly marked as "ALWAYS" and "MANDATORY" in the project guidelines. The missing optimizations could cause unnecessary re-renders and performance issues.Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant User participant NodePage as Node.tsx participant NodeNetwork as NodeNetwork.tsx participant NetworkAPI as networkApi participant Redux as Redux Store participant Components as UI Components User->>NodePage: Navigate to node page with network tab NodePage->>NodePage: Parse activeTabId from URL NodePage->>NodePage: Filter available tabs based on node capabilities alt Network tab selected NodePage->>NodeNetwork: Render NodeNetwork component NodeNetwork->>Redux: Get auto refresh interval NodeNetwork->>Redux: Get problem filter state NodeNetwork->>NetworkAPI: useGetNetworkInfoQuery(tenantName) NetworkAPI-->>NodeNetwork: Return network data or loading state alt Data loading NodeNetwork->>Components: Show loader Components-->>User: Display loading spinner else Data loaded successfully NodeNetwork->>NodeNetwork: Find current node and peers NodeNetwork->>NodeNetwork: Group nodes by type and rack NodeNetwork->>Components: Render controls (filters, checkboxes) NodeNetwork->>Components: Render current node section NodeNetwork->>Components: Render network peers section Components-->>User: Display network visualization else No network data NodeNetwork->>Components: Show placeholder with network icon Components-->>User: Display 'No network peers found' message else Error occurred NodeNetwork->>Components: Show ResponseError Components-->>User: Display error message end end User->>NodeNetwork: Toggle problem filter NodeNetwork->>Redux: Dispatch changeFilter action Redux-->>NodeNetwork: Update filter state NodeNetwork->>Components: Re-render filtered nodes Components-->>User: Update display based on filter User->>NodeNetwork: Toggle show ID checkbox NodeNetwork->>NodeNetwork: Update showId state NodeNetwork->>Components: Re-render nodes with/without IDs Components-->>User: Toggle node ID visibility User->>NodeNetwork: Toggle show racks checkbox NodeNetwork->>NodeNetwork: Update showRacks state NodeNetwork->>Components: Re-render nodes with rack grouping Components-->>User: Toggle rack visualization User->>Components: Hover over node Components->>Redux: Dispatch showTooltip action Redux-->>User: Display node tooltip User->>Components: Mouse leave node Components->>Redux: Dispatch hideTooltip action Redux-->>User: Hide tooltip